-
Notifications
You must be signed in to change notification settings - Fork 489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add texture scale mode #1444
base: master
Are you sure you want to change the base?
Add texture scale mode #1444
Conversation
src/sdl2/render.rs
Outdated
type Error = (); | ||
|
||
fn try_from(n: u32) -> Result<Self, Self::Error> { | ||
Ok(match unsafe { transmute(n) } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge no-no, it will crash badly if n
is not a safe value, even though the function is a try_from
, which should simply output a Err
on invalid value.
Read the implications of transmute with enums and fix this PR, this is not the only time you're using it, and I don't think you should be using it even once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to make this change. Quick question first, should these existing instances be changed as well? My intent was to follow the existing style.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked on the playground, they are all big mistakes and should be fixed ASAP:
Try in debug and release modes, it's wildly different behaviors, and never what's expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in: aa858db
otherwise texture copy to a smaller destination can look quite bad